-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(feedback): enforce a max message size from all sources #79326
base: master
Are you sure you want to change the base?
Conversation
tags={ | ||
"is_spam": is_message_spam, | ||
"pow2_size_bucket": 2 ** math.ceil(math.log2(len(feedback_message))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Limits the granularity of this tag. I'd rather do this for viewing in datadog, instead of logging each msg size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cant we just use a distribution metric type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yes!
This PR has a migration; here is the generated SQL for --
-- Alter field comments on userreport
--
-- (no-op) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migration lgtm
… in one file + rename/comment
src/sentry/ingest/userreport.py
Outdated
metrics.incr( | ||
"feedback.large_message", | ||
tags={ | ||
"pow2_size_bucket": 2 ** math.ceil(math.log2(len(report["comments"]))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here. can we just make it a distribution
Closes #76298
Closes SENTRY-3B86
Decided on a limit of 4096, generously below the LLM request, postgres, and kafka size limits described in the ticket. Messages that are too large will be truncated (or rejected, for crash report modal) and skip spam detection, auto-marking as spam.
Includes a small refactor of
create_feedback.py
, moving stuff around and commenting a bit.Renamedauto_ignore_spam_feedback
toset_feedback_ignored
.